feat: add runtime in-page channel events - #358
Conversation
◈ PR Lens
Architecture 
5 components touched across 2 lanes. Inside the changed components — 1 viewComponent view — In-page channel bridge Internal endpoint wiring, function registry, and diagnostics for in-page events. 
Data flow 
The other flows — 1 sequence
Drill down
|
|
@posva is attempting to deploy a commit to the NuxtLabs Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
🟡 Changes recommended
defineChannelFunction() can unintentionally allow missing handlers when type is omitted due to generic inference, weakening type safety for the helper API.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds runtime event subscription and a clearer emission API to the in-page channel so panels/page scripts can dynamically subscribe/unsubscribe to in-page events, while keeping compatibility with the existing callEvent() API.
Changes:
- Introduces typed
channel.emit()plus runtimechannel.on()(returning an unsubscribe function) on both page-script and panel endpoints; keepscallEvent()as a deprecated alias. - Allows
type: 'event'function declarations to omithandler, enabling purely runtime-driven listeners. - Updates tests, type tests, docs, and the a11y devframe to use
emit()and validate the new runtime subscription behavior.
File summaries
| File | Description |
|---|---|
| tests/snapshots/tsnapi/devframe/in-page-channel.snapshot.d.ts | Updates public type snapshots to include emit()/on() and relaxed event handler requirements. |
| plugins/a11y/app/lib/channel.ts | Migrates a11y panel-side event sending from callEvent() to emit(). |
| packages/devframe/src/in-page-channel/types.ts | Updates public types/docs for protocol semantics; enables event declarations without handlers; adds emit()/on() to endpoint interfaces. |
| packages/devframe/src/in-page-channel/types.test-d.ts | Adds type-level tests for event-without-handler and runtime on() typing. |
| packages/devframe/src/in-page-channel/panel.ts | Adds emit() and on() to the panel endpoint implementation; makes functions required at runtime. |
| packages/devframe/src/in-page-channel/page-script.ts | Adds emit() and on() to the page-script endpoint implementation; deprecates callEvent() to alias emit(). |
| packages/devframe/src/in-page-channel/internal.ts | Extends the local registry to support runtime listeners via on() and resolves handlers accordingly. |
| packages/devframe/src/in-page-channel/index.ts | Adjusts defineChannelFunction generics (ARGS default). |
| packages/devframe/src/in-page-channel/in-page-channel.test.ts | Updates runtime tests to validate emit() + runtime subscription/unsubscribe behavior. |
| docs/content/8.references/5.browser-api.md | Adds an in-page channel endpoint API reference table. |
| docs/content/1.guide/12.in-page-channel.md | Updates guide examples and wording to use emit() + runtime on() subscriptions. |
Review details
Suppressed comments (1)
packages/devframe/src/in-page-channel/index.ts:40
defineChannelFunction()leavesTYPEwithout a default, so calls that omittypecan inferTYPEas the fullInPageFunctionTypeunion. With the new conditional handler optionality for events, that can accidentally makehandleroptional (e.g.defineChannelFunction({ name: 'x' })becomes type-valid), weakening the helper’s compile-time safety.
export function defineChannelFunction<
NAME extends string,
TYPE extends InPageFunctionType,
ARGS extends any[] = [],
RETURN = void,
const AS extends RpcArgsSchema | undefined = undefined,
const RS extends RpcReturnSchema | undefined = undefined,
>(
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| NAME extends string, | ||
| TYPE extends InPageFunctionType, | ||
| ARGS extends any[], | ||
| ARGS extends any[] = [], |
There was a problem hiding this comment.
needed because handler is optional if type is event
There was a problem hiding this comment.
🟡 Changes recommended
The new runtime listener registry currently allows subscribing to undeclared method names and can silently treat missing non-event handlers as no-ops, which risks bypassing intended validation and masking misconfiguration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/devframe/src/in-page-channel/page-script.ts:67
options.functionsis assumed present at runtime, but if a non-TypeScript consumer passes{}/ omits it,Object.entries(options.functions)will throw a low-signal error. A small runtime assertion would make misconfiguration easier to diagnose.
packages/devframe/src/in-page-channel/panel.ts:66options.functionsis now required by the types, but at runtime (JS usage)Object.entries(options.functions)will throw a genericCannot convert undefined or null to objectif it’s missing. Adding an explicit runtime assertion here would produce a clearer error for consumers who aren’t typechecked.
- Files reviewed: 10/11 changed files
- Comments generated: 2
- Review effort level: Lite
| on(name, listener) { | ||
| let registered = listeners.get(name) | ||
| if (!registered) { | ||
| registered = new Set() | ||
| listeners.set(name, registered) | ||
| } | ||
| registered.add(listener) | ||
| return () => { |
| if (definition?.args?.length) | ||
| await validateArgs(definition.name, definition.args, args) | ||
| const result = await definition.handler(...args) | ||
| if (definition.jsonSerializable) | ||
| const result = await definition?.handler?.(...args) | ||
| for (const listener of [...(listeners.get(name) ?? [])]) | ||
| listener(...args) |
| ...args: FnArgs<PanelFunctions<P>[K]> | ||
| ) => void | ||
| /** Subscribe to an event emitted by a panel. Returns an unsubscribe function. */ | ||
| on: <K extends keyof PageScriptFunctions<P> & string>( |
There was a problem hiding this comment.
this is also wrong: it's probably allowing too many, it should only allow those of type: 'event'
|
I'm realizing some stuff is still not good enough, so marking as draft |
| navigation: | ||
| icon: i-lucide-globe | ||
| description: 'Lookup tables for the browser side: connectDevframe options, RPC client events, connection statuses, and in-page channel error codes.' | ||
| description: 'Lookup tables for the browser side: connectDevframe options, RPC client events, connection statuses, and in-page channels.' |
There was a problem hiding this comment.
so many useless edits on this one... I'm reverting them
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete issues to address (docs wording/example correctness and a runtime Object.entries(options.functions) crash hazard) before it’s safe to approve.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
packages/devframe/src/in-page-channel/types.ts:48
- Same issue as above: panel “events” are inferred from
voidreturn types, which will also treat any() => voidrequest/response action as an event. If you plan to keep inference for now, consider at least documenting this limitation prominently in the in-page channel types/docs.
/**
* Panel functions whose resolved return type marks an event.
* @internal
*/
type PanelFunctionsEvents<P extends InPageChannelProtocol> = {
[K in keyof PanelFunctions<P> as FnReturn<PanelFunctions<P>[K]> extends void ? K : never]: PanelFunctions<P>[K]
}
- Files reviewed: 12/13 changed files
- Comments generated: 5
- Review effort level: Lite
| const registry = createLocalFunctionRegistry(codec) | ||
| for (const [fnName, definition] of Object.entries(options.functions ?? {})) | ||
| for (const [fnName, definition] of Object.entries(options.functions)) | ||
| registry.register({ ...definition, name: fnName }) |
| for (const [fnName, definition] of Object.entries(options.functions)) | ||
| registry.register({ ...definition, name: fnName }) |
| /** | ||
| * Page-script functions whose resolved return type marks an event. | ||
| * @internal | ||
| */ | ||
| type PageScriptFunctionsEvents<P extends InPageChannelProtocol> = { | ||
| [K in keyof PageScriptFunctions<P> as FnReturn<PageScriptFunctions<P>[K]> extends void ? K : never]: PageScriptFunctions<P>[K] | ||
| } |
| ``` | ||
|
|
||
| `callEvent` on the page script is 1:N: it fans out to every connected panel, and panels that don't implement the function ignore it. Request/response *to* a panel goes through an explicit peer handle: `channel.panels[0].call('flash', '…')`. | ||
| `emit` on the page-script endpoint is 1:N: it fans out to every connected panel endpoint. Request/response *to* a panel goes through an explicit peer handle: `pageChannel.panels[0].call('flash', '…')`. |
| navigation: | ||
| icon: i-lucide-globe | ||
| description: 'Lookup tables for the browser side: connectDevframe options, RPC client events, connection statuses, and in-page channel error codes.' | ||
| description: 'Lookup tables for the browser side: connectDevframe options, RPC client events, connection statuses, and in-page channels error codes.' |
I realized that I need to listen to events dynamically, so the current setup doesn't work. I also noticed that callEvent could use a better name like
emit()channel.emit()+channel.on();on()returns an unsubscribe function.type: 'event'functions to omithandlercallEvent()as a deprecated compatibility alias.This makes me think that
channel.eventsmight also be a bit confusing right now, I did try to go for this initiallyAnother thought: this implementation infer events from the types directly: functions that return
void. This is not correct as an action can return void and just be used to catch errors or timeouts. I think this PR is big enough to leave that to another one but here are 2 possibilities:Adding a different
eventsproperty (and maybe reworking the shape ofInPageChannelProtocolto also havefunctions):A type marker: